Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better traceback for bad imports in custom path #735

Merged
merged 1 commit into from Nov 22, 2018

Conversation

dset0x
Copy link

@dset0x dset0x commented May 22, 2015

No description provided.

except ImportError:
# support importing modules not yet set up by the parent module
# (or package for that matter)
module = import_string(module_name)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@RonnyPfannschmidt
Copy link
Contributor

i suspect a potential use case lingering in lazy code loaders that inject modules into sys.modules at their own import

on closer examination the internal logic is grown a bit strange and should probably do a different loop altogether

@davidism
Copy link
Member

davidism commented Nov 21, 2018

We can preserve the "not set up by parent" case by saving the exception and raising it in the AttributeError block. However, this just adds more complexity to a code path no one can reproduce. And it makes the chained traceback in Python 3 look like it's coming from the wrong place. I'm inclined to merge this PR as-is, leaving this diff as a note in case someone comes up with a failing test case.

diff --git a/werkzeug/utils.py b/werkzeug/utils.py
index 5c8f5125..b3e859f7 100644
--- a/werkzeug/utils.py
+++ b/werkzeug/utils.py
@@ -423,10 +423,17 @@ def import_string(import_name, silent=False):
             return sys.modules[import_name]
 
         module_name, obj_name = import_name.rsplit('.', 1)
-        module = __import__(module_name, globals(), locals(), [obj_name])
+        preserved_import_error = None
+        try:
+            module = __import__(module_name, globals(), locals(), [obj_name])
+        except ImportError as e:
+            preserved_import_error = e
+            module = import_string(module_name)
         try:
             return getattr(module, obj_name)
         except AttributeError as e:
+            if preserved_import_error is not None:
+                raise preserved_import_error
             raise ImportError(e)
 
     except ImportError as e:

@davidism davidism merged commit 01f54c5 into pallets:master Nov 22, 2018
davidism added a commit that referenced this pull request Nov 22, 2018
@dset0x
Copy link
Author

dset0x commented Nov 22, 2018

Nearly three and a half years later, I still vividly recall the dozens of hours of pain that this was helping solve for the (now-deprecated) referenced module. Thanks for the merge. I hope it helps people get useful tracebacks.

@davidism
Copy link
Member

Sorry it took so long 😞 It came up in a much more recent issue, so it will still help. Thanks for working on it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants